-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support LSP diagnostic tags #9767
Conversation
These are defined as [`DiagnosticTag`] in the spec. Tags are an optional part of a diagnostic that can give some extra metadata. Currently the spec only contains `Unnecessary` and `Deprecated` and it gives some hints on how to theme those keys. We can introduce some new theme keys for these tags and fall back to the diagnostic's severity highlight. [`DiagnosticTag`]: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticTag
Sort of semiproductove input: This came up for. Either in an issue or somekind of PR. I know I also locked atcode like this. I can't remember more than that fir the life of me but maybe searching trough github may turn something up |
I think one more thing I remebrr: What pyroght is doing may or may not be non standard compliant. I think that is the direction the discussion around these tags went last time |
(For context this is related to #9765) Yeah I believe pyright should still be sending these diagnostics regardless of if the client supports tags, though I'm not sure if the spec says it explicitly. I might send a PR upstream to fix that. Regardless though I think it would be nice to take advantage of the distinct highlights we get from tags when a language server sends them |
I submitted that change upstream: microsoft/pyright#7380 |
I submitted this only for pyright and I'm not aware of any other language server that takes advantage of this feature. Pyright is insistent on their interpretation of the spec despite the considerable discussion against it: microsoft/pyright#1118 So I'm closing this in protest of pyright's IMO incorrect interpretation and IMO bad-faith discussion. If we can find another language server that uses this feature in the spirit the spec suggests then we can re-open but I don't see a point currently. We can do as the pyright maintainer suggests and guide people away from pyright and towards pylance instead |
It's a shame to see the outcome of microsoft/pyright#1118. Thanks for jumping on this though @the-mikedavis I appreciated the effort. A silver lining is that I kinda solved my issue by adding I will say tangentially that I really love how neovim/neovide can dim out entire blocks of unreachable code. It makes the diagnostic a lot more visual and illustrative where it makes sense. I'm not sure if killing this PR prevents helix from being able to do this or not, but if helix can do this it makes sense to me to include tags in the clientInfo to convey that to the server. Here's a cool example using Pyright on neovim/neovide: |
Yeah maybe closing this is a little hasty. I suppose we can also "protest" pyright's behavior by just implementing the spec correctly :P (as Neovim does) I totally overlooked that rust-analyzer uses this part of the spec (correctly): https://github.com/rust-lang/rust-analyzer/blob/79e0fee6a30a5f563e9b709cc5959694709e19c4/crates/rust-analyzer/src/diagnostics/to_proto.rs#L339-L357 |
These are defined as
DiagnosticTag
in the spec. Tags are an optional part of a diagnostic that can give some extra metadata. Currently the spec only containsUnnecessary
andDeprecated
and it gives some hints on how to theme those keys. We can introduce some new theme keys for these tags and fall back to the diagnostic's severity highlight.